Fix or silence `ruff check` warnings; add `ruff check` to Github Actions by roryyorke · Pull Request #246 · python-control/Slycot

roryyorke · 2025-01-19T11:27:35Z

Is this worth it at all?

Are the particular fixes I've made (especially # noqa ones) reasonable?

Removed unused imports. Removed unused variables in test: these were cut-and-paste from other tests, and not needed for the tests in questions. Other minor fixes.

bnavigator · 2025-01-30T19:21:43Z

I like the general approach! Thanks @roryyorke!

I would prefer not to override E741 (ambigous variable name) on single lines. I'd rather just switch it off globally. We use the single letter variable names where SLICOT uses them.

bnavigator · 2025-01-30T19:23:56Z

slycot/analysis.py



-def ag08bd(l,n,m,p,A,E,B,C,D,equil='N',tol=0.0,ldwork=None):
+def ag08bd(l,n,m,p,A,E,B,C,D,equil='N',tol=0.0,ldwork=None): # noqa: E741


Just switch off E741 in a global option in pyproject.toml

[tool.ruff.lint] ignore = [ "E741" ]

Done. If it weren't backward incompatible I'd be tempted to change l to ell, but so it goes.

roryyorke · 2025-02-01T04:58:46Z

From what I see, all failures are on macos, and I think all macos jobs fail. I checked two at random, and the failure is in python-control TestStateSpace.test_pow:

___________________________ TestStateSpace.test_pow ____________________________
  
  self = <control.tests.statesp_test.TestStateSpace object at 0x10b791ff0>
  sys222 = StateSpace(
  array([[ 4.,  1.],
         [ 2., -3.]]),
  array([[ 5.,  2.],
         [-3., -3.]]),
  array([[ 2., -4.],
         [ 0.,  1.]]),
  array([[ 3.,  2.],
         [ 1., -1.]]),
  states=2, outputs=2, inputs=2)
  sys322 = StateSpace(
  array([[-3.,  4.,  2.],
         [-1., -3.,  0.],
         [ 2.,  5.,  3.]]),
  array([[ 1.,  4.],
         [-3., ..., -3.],
         [ 1.,  4.,  3.]]),
  array([[-2.,  4.],
         [ 0.,  1.]]),
  name='sys322', states=3, outputs=2, inputs=2)
  
      @slycotonly
      def test_pow(self, sys222, sys322):
          """Test state space powers."""
          for sys in [sys222, sys322]:
              # Power of 0
              result = sys**0
              expected = StateSpace([], [], [], np.eye(2), dt=0)
              np.testing.assert_allclose(expected.A, result.A)
              np.testing.assert_allclose(expected.B, result.B)
              np.testing.assert_allclose(expected.C, result.C)
              np.testing.assert_allclose(expected.D, result.D)
              # Power of 1
              result = sys**1
              expected = sys
              np.testing.assert_allclose(expected.A, result.A)
              np.testing.assert_allclose(expected.B, result.B)
              np.testing.assert_allclose(expected.C, result.C)
              np.testing.assert_allclose(expected.D, result.D)
              # Power of -1 (inverse of biproper system)
              # Testing transfer function representations to avoid the
              # non-uniqueness of the state-space representation. Once MIMO
              # canonical forms are supported, can check canonical state-space
              # matrices instead.
              result = (sys * sys**-1).minreal()
              expected = StateSpace([], [], [], np.eye(2), dt=0)
              assert _tf_close_coeff(
                  ss2tf(expected).minreal(),
                  ss2tf(result).minreal(),
              )
              result = (sys**-1 * sys).minreal()
              expected = StateSpace([], [], [], np.eye(2), dt=0)
  >           assert _tf_close_coeff(
                  ss2tf(expected).minreal(),
                  ss2tf(result).minreal(),
              )
  E           assert False
  E            +  where False = _tf_close_coeff(TransferFunction(\n[[array([1.]), array([0.])],\n [array([0.]), array([1.])]],\n[[array([1.]), array([1.])],\n [array([1.]), array([1.])]],\noutputs=2, inputs=2), TransferFunction(\n[[array([1.]), array([-2.76395297e-15, -1.35541257e-13,  9.01576541e-13])],\n [array([0.]), array([1.])]],\n[[array([1.]), array([  1.,   3.,  -9., -41.])],\n [array([1.]), array([1.])]],\noutputs=2, inputs=2))
  E            +    where TransferFunction(\n[[array([1.]), array([0.])],\n [array([0.]), array([1.])]],\n[[array([1.]), array([1.])],\n [array([1.]), array([1.])]],\noutputs=2, inputs=2) = minreal()
  E            +      where minreal = TransferFunction(\n[[array([1.]), array([0.])],\n [array([0.]), array([1.])]],\n[[array([1.]), array([1.])],\n [array([1.]), array([1.])]],\noutputs=2, inputs=2).minreal
  E            +        where TransferFunction(\n[[array([1.]), array([0.])],\n [array([0.]), array([1.])]],\n[[array([1.]), array([1.])],\n [array([1.]), array([1.])]],\noutputs=2, inputs=2) = ss2tf(StateSpace(\narray([], shape=(0, 0), dtype=float64),\narray([], shape=(0, 2), dtype=float64),\narray([], shape=(2, 0), dtype=float64),\narray([[1., 0.],\n       [0., 1.]]),\nstates=0, outputs=2, inputs=2))
  E            +    and   TransferFunction(\n[[array([1.]), array([-2.76395297e-15, -1.35541257e-13,  9.01576541e-13])],\n [array([0.]), array([1.])]],\n[[array([1.]), array([  1.,   3.,  -9., -41.])],\n [array([1.]), array([1.])]],\noutputs=2, inputs=2) = minreal()
  E            +      where minreal = TransferFunction(\n[[array([  1.,   3.,  -9., -41.]), \n  array([-2.76395297e-15, -1.35541257e-13,  9.01576541e-13])],\n ....,   3.,  -9., -41.]), array([  1.,   3.,  -9., -41.])],\n [array([1.]), array([1., 0., 0., 0.])]],\noutputs=2, inputs=2).minreal
  E            +        where TransferFunction(\n[[array([  1.,   3.,  -9., -41.]), \n  array([-2.76395297e-15, -1.35541257e-13,  9.01576541e-13])],\n ....,   3.,  -9., -41.]), array([  1.,   3.,  -9., -41.])],\n [array([1.]), array([1., 0., 0., 0.])]],\noutputs=2, inputs=2) = ss2tf(StateSpace(\narray([[  4.86563613,  -0.22114886, -14.38268526],\n       [  3.86388385,  -1.64824725, -16.2652334 ],\n    ... [0.00000000e+00, 2.59786829e-16, 3.85014366e-15]]),\narray([[1., 0.],\n       [0., 1.]]),\nstates=3, outputs=2, inputs=2))
  
  control/tests/statesp_test.py:596: AssertionError

roryyorke · 2025-02-01T05:04:09Z

not all macos tests fail; many pass, but 7 fail.

bnavigator · 2025-02-01T08:41:56Z

Failing macOS tests are unrelated. Thanks @roryyorke

roryyorke added 2 commits January 19, 2025 11:52

Run "ruff check" on source in Github action

f305cc5

Make source ruff check-clean

8978584

Removed unused imports. Removed unused variables in test: these were cut-and-paste from other tests, and not needed for the tests in questions. Other minor fixes.

roryyorke requested a review from bnavigator January 19, 2025 11:27

roryyorke mentioned this pull request Jan 19, 2025

pyflakes (pylint, ruff, ....) clean-up, CI? python-control/python-control#1101

Open

bnavigator requested changes Jan 30, 2025

View reviewed changes

Disable Ruff 3741 project-wide

f1cb889

bnavigator merged commit bbfe718 into python-control:master Feb 1, 2025
64 of 71 checks passed

roryyorke mentioned this pull request Sep 7, 2025

Release 0.6.1 #248

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix or silence `ruff check` warnings; add `ruff check` to Github Actions#246

Fix or silence `ruff check` warnings; add `ruff check` to Github Actions#246
bnavigator merged 3 commits intopython-control:masterfrom
roryyorke:ruff-clean

roryyorke commented Jan 19, 2025

Uh oh!

bnavigator commented Jan 30, 2025

Uh oh!

bnavigator Jan 30, 2025

Uh oh!

roryyorke Feb 1, 2025

Uh oh!

roryyorke commented Feb 1, 2025

Uh oh!

roryyorke commented Feb 1, 2025

Uh oh!

bnavigator commented Feb 1, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants



		def ag08bd(l,n,m,p,A,E,B,C,D,equil='N',tol=0.0,ldwork=None):
		def ag08bd(l,n,m,p,A,E,B,C,D,equil='N',tol=0.0,ldwork=None): # noqa: E741

Conversation

roryyorke commented Jan 19, 2025

Uh oh!

bnavigator commented Jan 30, 2025

Uh oh!

bnavigator Jan 30, 2025

Choose a reason for hiding this comment

Uh oh!

roryyorke Feb 1, 2025

Choose a reason for hiding this comment

Uh oh!

roryyorke commented Feb 1, 2025

Uh oh!

roryyorke commented Feb 1, 2025

Uh oh!

bnavigator commented Feb 1, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants